Skip to content

Conversation

@a-zw
Copy link
Contributor

@a-zw a-zw commented Jan 21, 2026

📌 Description

Provides a Bazel command for source links to make source dependency explicit.
Use this target in docs() instead of implicitly scanning the git repo.
This allows to have source links even for other modules.

🚨 Impact Analysis

  • This change does not violate any tool requirements and is covered by existing tool requirements
  • This change does not violate any design decisions
  • Otherwise I have created a ticket for new tool qualification

✅ Checklist

  • Added/updated documentation for new or changed features
  • Added/updated tests to cover the changes
  • Followed project coding standards and guidelines

@github-actions
Copy link

github-actions bot commented Jan 21, 2026

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run //src:license-check

Status: ⚠️ Needs Review

Click to expand output
[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.3.0) and connecting to it...
INFO: Invocation ID: 7d2dfee6-17fb-407c-afd8-0ca506b708f7
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: src
Loading: 0 packages loaded
    currently loading: src
Analyzing: target //src:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //src:license-check (1 packages loaded, 0 targets configured)

Analyzing: target //src:license-check (70 packages loaded, 9 targets configured)

Analyzing: target //src:license-check (119 packages loaded, 806 targets configured)

Analyzing: target //src:license-check (135 packages loaded, 2651 targets configured)

Analyzing: target //src:license-check (140 packages loaded, 2700 targets configured)

INFO: Analyzed target //src:license-check (143 packages loaded, 4716 targets configured).
[10 / 13] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar; 0s disk-cache
INFO: Found 1 target...
Target //src:license.check.license_check up-to-date:
  bazel-bin/src/license.check.license_check
  bazel-bin/src/license.check.license_check.jar
INFO: Elapsed time: 15.244s, Critical Path: 0.41s
INFO: 13 processes: 4 disk cache hit, 9 internal.
INFO: Build completed successfully, 13 total actions
INFO: Running command line: bazel-bin/src/license.check.license_check src/formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

@github-actions
Copy link

The created documentation from the pull request is available at: docu-html

import sys
from pathlib import Path

from src.extensions.score_source_code_linker.generate_source_code_links_json import (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might need to either make this function public or make a public counterpart.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure yet. Eventually, we should be able to extract it from the Sphinx extension, because the extension shall only consume the json files.

Copy link
Contributor

@MaximilianSoerenPollak MaximilianSoerenPollak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks great from my side.
Script part is simple and concise.

Just had some questions regarding some of the things done as I don't understand them.

@a-zw a-zw changed the title Provide //:sourcelinks_json Source links as Bazel target Jan 22, 2026
@a-zw a-zw force-pushed the sourcelinks_json branch 2 times, most recently from d781b67 to 529deb8 Compare January 22, 2026 16:12
@a-zw a-zw marked this pull request as ready for review January 22, 2026 16:40
Copy link
Contributor

@MaximilianSoerenPollak MaximilianSoerenPollak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall great stuff, thanks for the work.

Just couple comments / questions.

Comment on lines +28 to +31
In you ``BUILD`` files, you specify which source files to scan
with ``filegroup`` or ``glob`` or whatever Bazel mechanism you prefer.
Then, you use the ``sourcelinks_json`` rule to scan those files.
Finally, pass the scan results to the ``docs`` rule as ``sourcelinks`` attribute.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly it was requested that we use non direct language (like you etc.)
Though I don't care too much about it personally.
What's the opinion of the others?

Here is a suggestion how that could look like:

Suggested change
In you ``BUILD`` files, you specify which source files to scan
with ``filegroup`` or ``glob`` or whatever Bazel mechanism you prefer.
Then, you use the ``sourcelinks_json`` rule to scan those files.
Finally, pass the scan results to the ``docs`` rule as ``sourcelinks`` attribute.
In the ``BUILD`` files, it can be specified which source files should be scanned.
This can be achieved with ``filegroup``, ``glob`` or whatever Bazel mechanism is prefered.
Then, the ``sourcelinks_json`` rule is used to scan those files.
Finally, the scanned results need to be passed into ``docs`` rule as ``sourcelinks`` attribute.

@a-zw a-zw force-pushed the sourcelinks_json branch from 71b7a05 to cbb7a3f Compare January 23, 2026 14:49
@a-zw a-zw force-pushed the sourcelinks_json branch from 8eae746 to 502f550 Compare January 23, 2026 16:27
@AlexanderLanin
Copy link
Member

  1. can you make sure the documentation explains that it must be added to all BUILD files and contain ALL files with links, even bazel files or git files or config files or whatever where links can appear. Not just source code.
  2. this will remove source code links from pure sphinx runs and from vs code preview, right?

# *******************************************************************************

"""
Merge multiple sourcelinks JSON files into a single JSON file.
Copy link
Member

@AlexanderLanin AlexanderLanin Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scripts directory is only for local development (linters; super confusing) so far.

This is the bazel side of src/extensions/score_source_code_linker? I would still put it to src/extensions/score_source_code_linker/bazel_prebuild_step?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add README.md files to any new directory explaining what that is and a rough sketch of the architecture?

# *******************************************************************************

"""
Merge multiple sourcelinks JSON files into a single JSON file.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add README.md files to any new directory explaining what that is and a rough sketch of the architecture?


merged = []
for json_file in args.files:
if json_file.exists():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no warning on not exists?

if json_file.exists():
with open(json_file) as f:
data = json.load(f)
if isinstance(data, list):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no warning on else?

srcs = glob(
[
"*.py",
"*.yaml",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's super easy to forget some file here. Is a missing source code link safety relevant?

@RolandJentschETAS do you know?

"tests/**/*.rst",
"tests/**/*.yaml",
],
] + ["tests/rst/conf.py"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
] + ["tests/rst/conf.py"],
"tests/rst/conf.py"],

Why do we need to add this file now?

get_cache_filename(outdir, "score_source_code_linker_cache.json")
)
source_code_links_json = os.environ.get("SCORE_SOURCELINKS")
if not source_code_links_json:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain this if? Maybe in some overarching concept? Maybe in a separate PR ;-)

"//src/extensions/score_sphinx_bundle:all_sources",
"//src/extensions/score_sync_toml:all_sources",
"//src/find_runfiles:all_sources",
"//src/helper_lib:all_sources",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No clever collection mechanism in bazel? This looks extremely error prone.

sourcelinks_json(
name = "sourcelinks_json",
srcs = [
"//src:all_sources",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all following items are already included in src:all_sources?

"@score_process//:needs_json",
],
source_dir = "docs",
sourcelinks = [":my_source_links"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

up to now docs() is defining internal targets like needs_json itself. All in one command. Can we do the same with sourcelinks_json?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Draft

Development

Successfully merging this pull request may close these issues.

3 participants